-
-
Notifications
You must be signed in to change notification settings - Fork 610
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make editable requirements use relative paths where appropriate. #507
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately this only works for the "happy case" where the current working directory happens to be the common root of the requirements file and the local packages.
The local packages path would need to be resolved against the path of the requirements file from which it was referenced. However from briefly looking at the code it seems to me that the path isn't available after parsing has finished. So I'm not quite sure how to solve this problem :/
It's also common to have a requirements file in a directory like I guess to cover this case we could just say 'it resolves to a relative path if it is in a sub-directory of the requirements file' and be done with it (providing we can fix the issue you described). People who need this fix can then move the files as needed to make it work? |
path = ireq.link.path | ||
if ireq.link.scheme == 'file' and is_subdirectory(os.getcwd(), path): | ||
# If the ireq.link is relative to the current directory then output a relative path | ||
path = 'file:' + os.path.join('.', os.path.relpath(path)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that there are Windows idiosyncrasies that would be taken care of if you used pathname2url (like in the original pip code you pointed at:
urllib_parse.urljoin('file:', urllib_request.pathname2url(path))
this should also help the failing Windows build.
|
||
@fixture | ||
def minimal_wheels_dir(): | ||
return os.path.join(os.path.split(__file__)[0], 'fixtures', 'minimal_wheels') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perfect
def format_requirement(ireq, marker=None): | ||
""" | ||
Generic formatter for pretty printing InstallRequirements to the terminal | ||
in a less verbose way than using its `__str__` method. | ||
""" | ||
if ireq.editable: | ||
line = '-e {}'.format(ireq.link) | ||
path = ireq.link.path | ||
if ireq.link.scheme == 'file' and is_subdirectory(os.getcwd(), path): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not certain that cwd is a correct base here. I could call pip-compile outside of the directory which contains the requirement file, and that would leave us with a non installable compiled requirements.txt. Am I mistaken in thinking that the base should always be relative to the directory of the input file ?
This branch implements support for using relative paths as requirement entries (in setup.cfg or in requirements.in). The base feature comes from a pip-tools PR (jazzband#507) by Tom Forbes. Thanks Tom! Prequ supports also non-editable URLs and therefore the feature needed some polishing. The changes in this branch should make the feature work nicely as a whole.
#204
When
pip-compile
is given a relative editable dependency, like-e ./some_module/
, it is resolved to the full absolute path bypip
under the hood. This is not what we want or need in most cases.As the path resolution happens inside
pip
and it might be a bit complicated to add a change in that project, this merge request works around the issue by modifying theformat_requirement
to check that if the editable dependency uses thefile:
scheme and if the location of the editable dependency exists within a subdirectory of the current working directory then we format the dependency using the relative path from the current working directory.I think this is the best place to add this change and requires less potential breakage and internal changes. I'm not too sure on if we should use the current directory or the path from the output file directory, but that's a small detail.
I also did a small refactoring of the tests by adding fixtures to return the paths of the packages within the
tests/fixtures/
directory rather than duplicating variousos.path.join
calls everywhere.Contributor checklist